Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat!: update ucanto to invocation spec compatible result type #272

Merged
merged 4 commits into from
Apr 4, 2023
Merged

feat!: update ucanto to invocation spec compatible result type #272

merged 4 commits into from
Apr 4, 2023

Conversation

Gozala
Copy link
Collaborator

@Gozala Gozala commented Apr 1, 2023

Overview

  • Updates result type to use ucan invocation spec compatible representation
  • At the library layer (except of the schemas which needs to deal with optional fields) we now enforce no null|undefined values in Results and push towards use of unit value {} instead.
    • Mostly this guides users into a better and more extensible API design.
    • Please note that false and "" are still allowed.
  • ⚠️ above changes caused breaking API change on .derives methods than no longer return true or Failure but instead return Result<{}, Failure>.
  • ⚠️ Error types no longer have error: true field which were in place to pattern match on ok / error.

@Gozala Gozala requested a review from gobengo April 1, 2023 08:34
gobengo
gobengo approved these changes Apr 3, 2023
View reviewed changes
packages/core/src/result.js
* @param {string} message
* @returns {{error:API.Failure, ok?:undefined}}
*/
export const fail = message => ({ error: new Failure(message) })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should I decide when to use error vs fail? Can a comment or @fileoverview explain?

packages/interface/src/capability.ts Show resolved Hide resolved
@gobengo
Copy link
Contributor

gobengo commented Apr 3, 2023 •
edited
Loading

This is a big change to use the { ok } or { error } result types explicitly, but I think it's for the best in the long run. It will be easier to maintain this API vs what was there before, where it would try to detect whether the handler fn returned something extending { error: true } or not

@Gozala Gozala changed the title feat: update ucanto to invocation spec compatible result type feat!: update ucanto to invocation spec compatible result type Apr 4, 2023
@Gozala Gozala merged commit b124ed8 into main Apr 4, 2023
@github-actions github-actions bot mentioned this pull request Apr 4, 2023
Merged
This was referenced Apr 11, 2023
Merged
Merged
Merged
@heyjay44 heyjay44 mentioned this pull request Apr 11, 2023
Closed
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gobengo gobengo gobengo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gozala @gobengo